Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BB-7106] Add new endpoint for cloning course #31794

Merged
merged 3 commits into from
Apr 9, 2024
Merged

[BB-7106] Add new endpoint for cloning course #31794

merged 3 commits into from
Apr 9, 2024

Conversation

pkulkark
Copy link
Contributor

Description

This PR add a new studio endpoint for the existing clone_course method, so that external applications can call it using the following API:

/api/v1/course_runs/clone/

And passing the source_course_id (of an existing course) and destination_course_id (of a non-existing course) in the request data, like below:

{
    "source_course_id": "course-v1:edX+DemoX+Demo_Course",
    "destination_course_id": "course-v1:new+TestX+Demo_Course_Clone",
}

Supporting information

OpenCraft Internal Jira ticket: BB-7106

Testing instructions

  1. Checkout this branch and provision devstack.
  2. Login to studio and select a course which you want to clone. Note the course id.
  3. Using a valid client ID and secret, obtain the oath access token (see ref)
  4. With the access token, make a POST api call to endpoint: <your-studio-url>/api/v1/course_runs/clone/, and pass the source_course_id and destination_course_id in the data params.
  5. Check your studio home and verify that the new course has been created with the destination_course_id you provided and contains the contents of the source_course_id.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core committer labels Feb 20, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Feb 20, 2023 •
edited
Loading

Thanks for the pull request, @pkulkark! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@pkulkark pkulkark mentioned this pull request Feb 22, 2023
Merged
@pkulkark pkulkark force-pushed the pooja/bb7106-add-endpoint-for-clone-course branch from 00b15a0 to 526557e Compare February 22, 2023 19:23
@pdpinch
Copy link
Contributor

pdpinch commented Feb 22, 2023

Is cloning a course the same as creating a "Re-Run Course" ?

@Cup0fCoffee
Copy link
Contributor

👍

  • I tested this:
    • tested on the local devstack
    • sent POST request with only source_course_id
    • sent POST request with only destination_course_id
    • sent POST request with both valid source_course_id and destination_course_id
    • sent POST request with both valid source_course_id and destination_course_id again to verify that it won't clone again
  • I read through the code
  • [N/A] I checked for accessibility issues

@pkulkark
Copy link
Contributor Author

@pdpinch "re-run course" uses the clone function underneath. The difference being the course ID would remain the same. Essentially it creates a new course with the same course ID and copies over the contents. The changes here expose the function as an api endpoint and allows you to create a new course (with different course ID) and copy over the contents without having to manually export and import it.

@mphilbrick211
Copy link

Hi @pkulkark - just seeing if there's any update on this?

@mphilbrick211
Copy link

Hi @pkulkark - just seeing if there's any update on this?

Hi @pkulkark just following up on this :)

@pkulkark
Copy link
Contributor Author

pkulkark commented Apr 4, 2023

@mphilbrick211 This is ready for review. Please let me know if you need any further information from me.

@mphilbrick211
Copy link

Hi @jristau1984 - would you mind reviewing this and merging if all looks good? Thanks!

@jristau1984
Copy link
Contributor

@mphilbrick211 I do not have availability to review this for at least the next several weeks.

@jristau1984 jristau1984 requested review from a team and removed request for jristau1984 April 25, 2023 15:32
@mphilbrick211
Copy link

Hi @pkulkark! While we wait for @jristau1984 for review, would you mind re-running the tests? We have a couple new ones that have popped up (shellcheck) that need to be run. Please let me know if you have any questions. Thanks!

@mphilbrick211
Copy link

Hi @pkulkark! While we wait for @jristau1984 for review, would you mind re-running the tests? We have a couple new ones that have popped up (shellcheck) that need to be run. Please let me know if you have any questions. Thanks!

Friendly ping on this @pkulkark :)

@pkulkark pkulkark force-pushed the pooja/bb7106-add-endpoint-for-clone-course branch from 526557e to ec50632 Compare May 12, 2023 14:19
@pkulkark
Copy link
Contributor Author

@mphilbrick211 Sure, Done.

@mphilbrick211
Copy link

Hi @openedx/teaching-and-learning! Is someone able to review/merge this for us? Thanks!

@xitij2000 xitij2000 mentioned this pull request Jun 27, 2023
Merged
@mphilbrick211
Copy link

Hi @pkulkark - looks like some additional tests have popped up!

@pkulkark pkulkark force-pushed the pooja/bb7106-add-endpoint-for-clone-course branch from ec50632 to 59117e8 Compare July 18, 2023 15:11
@pkulkark
Copy link
Contributor Author

@mphilbrick211 Thanks for the ping. I rebased it to latest master and ran all the tests.

@mphilbrick211
Copy link

Thanks, @pkulkark!

@openedx/teaching-and-learning - just flagging that this is all set for review. Thanks!

@mphilbrick211
Copy link

Hi @pkulkark - please see here re: the tests that have popped up above. I think if you rebase, it should be all set.

@openedx-webhooks
Copy link

Thanks for the pull request, @pkulkark! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@pkulkark pkulkark force-pushed the pooja/bb7106-add-endpoint-for-clone-course branch from 59117e8 to 79c4b4a Compare September 25, 2023 19:27
@pkulkark
Copy link
Contributor Author

@mphilbrick211 Sorry for the delay. I've rebased this now and all tests are green.

@Cup0fCoffee
Copy link
Contributor

@mphilbrick211 @jristau1984 The PR has been fixed and is ready for review.

@itsjeyd itsjeyd added core contributor PR author is a Core Contributor (who may or may not have write access to this repo). and removed core committer labels Feb 20, 2024
@farhaanbukhsh
Copy link
Member

@mphilbrick211 @pkulkark I can review this PR as CC :)

@Cup0fCoffee
Copy link
Contributor

@farhaanbukhsh FYI, I've picked this ticket up from Pooja and am currently the assignee on the PR.

Sure, please do review this when you have the time!

@mphilbrick211
Copy link

Thank you, @farhaanbukhsh!

@Cup0fCoffee
Copy link
Contributor

@farhaanbukhsh Just checking if you're still planning to review the PR?

@farhaanbukhsh
Copy link
Member

@Cup0fCoffee Yes it's on my list :)

farhaanbukhsh
farhaanbukhsh requested changes Mar 25, 2024
View reviewed changes
Copy link
Member

@farhaanbukhsh farhaanbukhsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Cup0fCoffee @pkulkark

I have reviewed the code and read through it, this works great, Kudos on the work done here. I have a slight concern and improvement. We should probably add a docstring to the clone method because if we don't we wouldn't know the difference between clone and rerun.

If we can add the documentation then we will know how to use it and when to use it and maybe substanciate with an example.

What do you say?

PS: @Cup0fCoffee your commit also has a typo can we correct that as well?

@itsjeyd itsjeyd added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Apr 3, 2024
@Cup0fCoffee Cup0fCoffee force-pushed the pooja/bb7106-add-endpoint-for-clone-course branch from 8953719 to cc0d250 Compare April 8, 2024 08:49
@Cup0fCoffee
Copy link
Contributor

@farhaanbukhsh Thank you for your suggestions. I fixed the typo and added docstring for the clone API endpoint.

@itsjeyd itsjeyd added waiting for eng review PR is ready for review. Review and merge it, or suggest changes. and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Apr 8, 2024
@farhaanbukhsh
Copy link
Member

@Cup0fCoffee can you rebase the branch please :)

@Cup0fCoffee Cup0fCoffee force-pushed the pooja/bb7106-add-endpoint-for-clone-course branch from cc0d250 to 7567706 Compare April 9, 2024 07:41
farhaanbukhsh
farhaanbukhsh approved these changes Apr 9, 2024
View reviewed changes
Copy link
Member

@farhaanbukhsh farhaanbukhsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • ✅ I tested this on the devstack.
  • ✅ I read through the code
  • ❌ I checked for accessibility issues
  • ✅ Includes documentation
  • ❌ I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@farhaanbukhsh farhaanbukhsh merged commit 3c7e162 into openedx:master Apr 9, 2024
66 checks passed
@farhaanbukhsh farhaanbukhsh deleted the pooja/bb7106-add-endpoint-for-clone-course branch April 9, 2024 09:35
@openedx-webhooks
Copy link

@pkulkark 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@itsjeyd itsjeyd removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Apr 9, 2024
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

KyryloKireiev pushed a commit to raccoongang/edx-platform that referenced this pull request Apr 24, 2024
@pkulkark @Cup0fCoffee @KyryloKireiev
feat: add new endpoint for cloning course (openedx#31794)
43f359b 
Co-authored-by: Maxim Beder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@farhaanbukhsh farhaanbukhsh farhaanbukhsh approved these changes

Assignees

@farhaanbukhsh farhaanbukhsh

Labels
core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U
Projects
Contributions
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.